ci(docs): check changed markdown links on pull requests#1139
ci(docs): check changed markdown links on pull requests#113913ernkastel wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a PR-triggered GitHub Actions workflow that runs a local markdown link checker on changed Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GHA as GitHub Actions (docs-links-pr)
participant Script as check-docs.sh
participant FS as Repository File System
participant CI as CI Result
PR->>GHA: opened/reopened/synchronize with .md changes
GHA->>GHA: git diff base...head -> filter and list .md files
alt markdown files present
GHA->>Script: run with --only-links --local-only and file paths
Script->>Script: parse files, skip fenced code, emit line_no<TAB>target
loop per extracted target
Script->>FS: check target exists
FS-->>Script: exists / missing
end
alt missing targets found
Script-->>CI: exit non-zero with "md_path:line_no -> target"
else
Script-->>CI: exit 0
end
else no markdown files
GHA-->>CI: skip link check
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/e2e-cloud-experimental/check-docs.sh (1)
224-230: Handle tilde fences (~~~) in fenced-code skipping.
extract_targetscurrently toggles fence state only for backtick fences, so links inside~~~fenced blocks can still be parsed as real links. Line 225 is the toggle point to broaden.Suggested patch
- if (/^\s*```/) { $in = !$in; next; } + if (/^\s*(```|~~~)/) { $in = !$in; next; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e-cloud-experimental/check-docs.sh` around lines 224 - 230, The fenced-code detection only toggles for backticks (the Perl one-liner in check-docs.sh that currently uses if (/^\s*```/) to flip $in), so add tilde fence support by changing that condition to match either ``` or ~~~ (i.e., update the Perl fence-toggle regex inside the extract/processing one-liner to /^\s*(```|~~~)/ so links inside ~~~ blocks are skipped as well).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docs-links-pr.yaml:
- Line 35: The current population of md_files captures all changed *.md
including vendored/generated paths; update the mapfile command that sets
md_files to exclude common non-doc directories by filtering out patterns like
node_modules, dist, vendor, build (e.g., modify the git diff pipeline that
produces md_files or append a grep -v -E '^(node_modules|dist|vendor|build)/'
before the sort) so md_files only contains real documentation markdown changes.
---
Nitpick comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 224-230: The fenced-code detection only toggles for backticks (the
Perl one-liner in check-docs.sh that currently uses if (/^\s*```/) to flip $in),
so add tilde fence support by changing that condition to match either ``` or ~~~
(i.e., update the Perl fence-toggle regex inside the extract/processing
one-liner to /^\s*(```|~~~)/ so links inside ~~~ blocks are skipped as well).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47af3454-3672-41b5-a1a6-40f5430603a9
📒 Files selected for processing (3)
.github/workflows/docs-links-pr.yamltest/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
bd5d7fc to
1ff2346
Compare
|
Addressed the actionable review feedback in Updates included:
Validation rerun:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 224-225: Replace the permissive fence-toggle logic in the perl
one-liner (the /^\s*(```|~~~)/ branch that flips $in) with a delimiter-aware
handler: detect fence openings via /^\s*(`{3,}|~{3,})/ and record the fence
character and length into $fch and $flen when entering ($in=1), and only exit
the fence (set $in=0 and clear $fch/$flen) when a closing fence uses the same
character and length >= $flen; update the assignments/tuples that set ($in,
$fch, $flen) accordingly so the link-scanning next/while blocks remain gated by
the refined $in state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed0e22e1-3e2a-47bf-b23d-a297ccdf6b0a
📒 Files selected for processing (3)
.github/workflows/docs-links-pr.yamltest/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/docs-links-pr.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- test/check-docs-links.test.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e-cloud-experimental/check-docs.sh`:
- Around line 225-235: The fence-closing check using the regex
/^\s*(`{3,}|~{3,})/ incorrectly treats lines like "```not-a-close" as closes;
update the logic in the block that uses the regex and variables ($in, $fch,
$flen) so that a closing fence is only accepted if the matched fence is followed
only by optional whitespace (i.e., nothing else on the line). Concretely, either
change the regex to assert end-of-line or only whitespace after the marker, or
after matching check the remainder of the line for non-whitespace characters
before flipping ($in, $fch, $flen) to close the fence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b784f8ab-e97b-46f5-92cf-76320a8b344d
📒 Files selected for processing (2)
test/check-docs-links.test.jstest/e2e/e2e-cloud-experimental/check-docs.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- test/check-docs-links.test.js
|
The current head is ready from my side, but the latest required checks are still in GitHub's |
Includes the follow-up doc checker fixes from the original branch: locale-stable link checking, tighter markdown link filtering, and regression coverage for mixed fence delimiters and trailing text on fence closers. Signed-off-by: 13ernkastel <[email protected]>
Head branch was pushed to by a user without write access
858c0da to
c059875
Compare
Summary
check-docs.shoutput so broken local links include the source line numberWhy
Issue #552 asks for markdown link checking in CI. The repo already had a useful checker in
test/e2e/e2e-cloud-experimental/check-docs.sh, but it only ran in broader E2E contexts and its broken-link output did not point back to the exact markdown line.This keeps the fix small by reusing the existing checker instead of introducing a second link-checking tool. The pull request workflow runs
--local-onlyon changed markdown files so review-time checks stay fast and avoid flaky network-driven failures.Validation
bash -n test/e2e/e2e-cloud-experimental/check-docs.shruby -e 'require "yaml"; puts YAML.load_file(".github/workflows/docs-links-pr.yaml")["name"]'npx vitest run test/check-docs-links.test.jsbash test/e2e/e2e-cloud-experimental/check-docs.sh --only-links --local-only README.mdCloses #552.
Summary by CodeRabbit
New Features
Tests
Chores
Signed-off-by: 13ernkastel [email protected]